-
-
Notifications
You must be signed in to change notification settings - Fork 286
Pub Workspaces #1185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Pub Workspaces #1185
Conversation
WalkthroughAdds a Dart workspace, updates CI workflow and build settings, adjusts many pubspecs/overrides and .gitignore rules, adds/updates Freezed-generated example files (and removes one), tweaks lint messages, and reorganizes dependency overrides and build.yaml include/exclude patterns. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor GH as GitHub Actions
participant M as Matrix(package × channel)
participant SDK as Flutter/Dart SDK
participant P as Package (working-directory = matrix.package)
GH->>M: start job per matrix entry
M->>SDK: setup SDK (channel)
M->>P: flutter pub ${{ matrix.dependencies }}
alt package on stable
M->>P: flutter format --set-exit-if-changed
end
alt package != packages/freezed_lint/example
M->>P: dart run build_runner build --delete-conflicting-outputs
else
note right of M: generator skipped for freezed_lint/example\n(step allowed to continue)
end
M->>P: flutter analyze --fatal-infos
alt pubspec contains flutter_test
M->>P: flutter test
else pubspec contains test
M->>P: dart test
end
alt package == packages/freezed_lint/example
M->>P: dart run custom_lint --fatal-infos
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Huh. Can't repro the formatting issue on master. |
59e2e04 to
d64d341
Compare
9844bd0 to
57ae4a1
Compare
| *.freezed.dart | ||
| *.g.dart | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change gitignores?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been a while, I honestly don't remember.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I remember now. This was in the example, if the code wasn't checked in the order that CI ran tripped something up. c5d83a7
Probably not strictly needed anymore, but feels like a nice QoL improvement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (5)
packages/freezed_lint/lib/src/missing_private_empty_ctor.dart (1)
14-15: Fix grammar & remove superfluous comma in diagnostic messageThe concatenated string currently reads
'Freezed classes containing methods, fields or accessors, requires a {0}'.
Grammatically this should be “require”, and the comma after accessors is unnecessary.- 'Freezed classes containing methods, fields or accessors, ' - 'requires a {0}', + 'Freezed classes containing methods, fields or accessors require a {0}',packages/freezed/example/lib/main.freezed.dart (1)
1-13: Same formatting directive conflictbenchmarks/lib/src/copy_with.freezed.dart (1)
1-13: Same formatting directive conflictpackages/freezed/example/lib/diagnosticable.freezed.dart (1)
1-13: Same formatting directive conflictpackages/freezed/example/lib/non_diagnosticable.freezed.dart (1)
1-1: Remove dart format width directive (same issue as in the other file).
🧹 Nitpick comments (15)
packages/freezed_lint/pubspec.yaml (1)
10-10: Minor formatting nitConsider inserting a blank line before
resolution:for readability and to match the style used in other pubspecs.sdk: ">=3.5.0 <4.0.0" + resolution: workspacepackages/freezed_lint/lib/src/missing_mixin.dart (1)
11-11: Grammar tweak in diagnostic message“mix in” is the verb form; the current text reads a little awkward.
- problemMessage: 'Freezed class should mixin {0}', + problemMessage: 'Freezed class should mix in {0}',pubspec_overrides.yaml (1)
6-18: Keep overrides temporary & monitor for upstream releasesMultiple overrides (e.g.
frontend_server_client,matcher,quiver) are bumping versions beyond whatbuild_runnercurrently requires.
That’s fine locally, but remember to drop these overrides once upstream packages publish compatible releases; long-lived overrides hide real dependency conflicts.Consider adding a TODO with a tracking issue to ensure cleanup.
packages/freezed_lint/.gitignore (1)
1-3: Re-consider ignoring.dart_tool/andpubspec.lockat the package levelRemoving those two entries means they will be committed unless the root‐level
.gitignorestill excludes them.
Both directories are generated artefacts and rarely belong in source control. Accidentally adding them clutters diffs, breaks reproducible builds, and can leak local-machine paths..packages +/.dart_tool pubspec.lock custom_lint.logPlease verify that the root
.gitignore(or workspace rules) already covers them; otherwise add them back here.packages/freezed_lint/example/pubspec.yaml (1)
2-5: Pin versions instead ofanyto keep example builds deterministicAll runtime/dev dependencies are declared as
any. In a workspace this means the exact versions float with the latest that satisfy transitive constraints, which can lead to:
- irreproducible CI results over time,
- breakages when a dependency publishes a breaking change under a new major version,
- noisy diffs in
pubspec.lockthat hamper bisects.Even for examples it is common to use caret-ranges (
^) or>= <constraints to document the versions known to compile.Consider:
- freezed_annotation: any + freezed_annotation: ^2.4.1(and similar for the other four dev-dependencies).
If you prefer floating versions locally, you can still lock them in CI with
--no-pubcaching orpub downgrade.packages/freezed_lint/example/analysis_options.yaml (1)
11-18: Suppressing four additional analyzer errors may hide real regressionsIgnoring
uri_does_not_exist,redirect_to_non_class,mixin_of_non_class, andunused_elementproject-wide is quite broad.
Unless these diagnostics are intentionally triggered by the example (e.g., to demonstrate a lint), they risk masking genuine mistakes as the code evolves.If the ignores are required for specific files, prefer file-scoped directives:
// ignore_for_file: uri_does_not_exist, redirect_to_non_classThat keeps the global configuration stricter and protects other sources.
Please double-check whether the new ignores are strictly necessary.
packages/freezed/example/lib/time_slot.dart (1)
8-11: Keep trailing comma to stabilise formatter & diffsFormatting the factory parameters on a single line without a trailing comma will reflow again whenever an import line length changes.
Adding a trailing comma letsdart formatconsistently break the args onto multiple lines, reducing future churn.-factory TimeSlot({TimeOfDay? start, TimeOfDay? end}) = _TimeSlot; +factory TimeSlot({ + TimeOfDay? start, + TimeOfDay? end, +}) = _TimeSlot;benchmarks/lib/src/copy_with.dart (1)
13-15: Same trailing-comma nit for constructorFor consistency with the rest of the file and to avoid formatter flip-flops:
-factory ModelWrapper({required Model model}) = _ModelWrapper; +factory ModelWrapper({ + required Model model, +}) = _ModelWrapper;packages/freezed/example/pubspec.yaml (1)
14-18: Pinbuild_runnerfor reproducible builds
build_runneris still un-versioned. Without a constraint, everypub getmay fetch a different minor/patch release – unexpected breakages do happen.build_runner: + ^2.4.9packages/freezed/example/lib/main.dart (3)
54-59: Reduce duplicate sample codeTwo consecutive
switchexamples test the sameunionExampleand print the same constant. Collapsing them keeps the example concise:- print(switch (unionExample) { - Loading _ => 'loading', - // voluntarily didn't handle error/complex cases - _ => 42, - }); // 42
64-70: Secondswitchalready covers all casesWith the first
switchpreserved, this one adds no extra showcase value. Consider deleting or replacing it with a different example (e.g. pattern-matching onUnion.error).
82-84: Minor readability nitMulti-line
- print( - const Union.error('Failed to fetch'), - ); // Union.error(message: Failed to fetch) + print(const Union.error('Failed to fetch')); // Union.error(message: Failed to fetch)benchmarks/pubspec.yaml (1)
14-18: Samebuild_runnerpinning advicebuild_runner: + ^2.4.9Keeps benchmark runs reproducible.
packages/freezed/example/lib/equals.freezed.dart (1)
1-13: Formatting directive conflicts in generated codeThe file has
// dart format width=80at line 1 but then// dart format offat line 13, which explains the poor formatting throughout the file. Generated code should either be properly formatted or consistently indicate that formatting is disabled.Consider updating the code generator to either:
- Remove the format width directive if formatting will be disabled
- Generate properly formatted code and remove the format-off directive
- Add a comment explaining why formatting is disabled for generated files
benchmarks/lib/src/equal.freezed.dart (1)
1-1: Consider removing the dart format width directive from generated code.The
dart format width=80directive at the top of a generated file is unusual and could cause issues if the code generator doesn't consistently apply this formatting width. Generated files typically shouldn't include formatting directives as they should be controlled by the generator configuration.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (35)
.github/workflows/build.yml(2 hunks).gitignore(1 hunks)benchmarks/.gitignore(0 hunks)benchmarks/lib/src/copy_with.dart(1 hunks)benchmarks/lib/src/copy_with.freezed.dart(2 hunks)benchmarks/lib/src/equal.freezed.dart(2 hunks)benchmarks/pubspec.yaml(1 hunks)packages/freezed/.gitignore(0 hunks)packages/freezed/example/.gitignore(0 hunks)packages/freezed/example/lib/diagnosticable.freezed.dart(1 hunks)packages/freezed/example/lib/equals.freezed.dart(1 hunks)packages/freezed/example/lib/main.dart(2 hunks)packages/freezed/example/lib/main.freezed.dart(1 hunks)packages/freezed/example/lib/main.g.dart(1 hunks)packages/freezed/example/lib/non_diagnosticable.freezed.dart(1 hunks)packages/freezed/example/lib/time_slot.dart(1 hunks)packages/freezed/example/lib/time_slot.freezed.dart(1 hunks)packages/freezed/example/pubspec.yaml(1 hunks)packages/freezed/example/test/diagnosticable_test.dart(1 hunks)packages/freezed/example/test/json_test.dart(1 hunks)packages/freezed/lib/src/templates/concrete_template.dart(0 hunks)packages/freezed/pubspec.yaml(3 hunks)packages/freezed/pubspec_overrides.yaml(0 hunks)packages/freezed_annotation/.gitignore(0 hunks)packages/freezed_annotation/pubspec.yaml(1 hunks)packages/freezed_lint/.gitignore(1 hunks)packages/freezed_lint/example/analysis_options.yaml(1 hunks)packages/freezed_lint/example/lib/missing_mixin.freezed.dart(0 hunks)packages/freezed_lint/example/pubspec.yaml(1 hunks)packages/freezed_lint/lib/src/missing_mixin.dart(1 hunks)packages/freezed_lint/lib/src/missing_private_empty_ctor.dart(1 hunks)packages/freezed_lint/pubspec.yaml(1 hunks)packages/freezed_lint/pubspec_overrides.yaml(0 hunks)pubspec.yaml(1 hunks)pubspec_overrides.yaml(1 hunks)
💤 Files with no reviewable changes (8)
- benchmarks/.gitignore
- packages/freezed_annotation/.gitignore
- packages/freezed/example/.gitignore
- packages/freezed/.gitignore
- packages/freezed_lint/pubspec_overrides.yaml
- packages/freezed/pubspec_overrides.yaml
- packages/freezed/lib/src/templates/concrete_template.dart
- packages/freezed_lint/example/lib/missing_mixin.freezed.dart
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
packages/freezed_lint/pubspec.yaml (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
packages/freezed_annotation/pubspec.yaml (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
packages/freezed_lint/lib/src/missing_private_empty_ctor.dart (1)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
packages/freezed_lint/lib/src/missing_mixin.dart (1)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
packages/freezed_lint/.gitignore (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
packages/freezed/pubspec.yaml (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
packages/freezed_lint/example/analysis_options.yaml (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
packages/freezed_lint/example/pubspec.yaml (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
pubspec.yaml (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
packages/freezed/example/pubspec.yaml (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
benchmarks/pubspec.yaml (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
pubspec_overrides.yaml (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
.github/workflows/build.yml (2)
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version upgrade in freezed_lint is blocked by invertase/dart_custom_lint#300 which handles the broader upgrade of analyzer, analyzer_plugin, and source_gen dependencies in custom_lint.
🔇 Additional comments (19)
.gitignore (1)
1-1: 👍.dart_toolnow correctly ignoredAdding the
.dart_tooldirectory to the root ignore list prevents thousands of transient build artefacts from polluting the repo and is consistent with the new workspace layout.packages/freezed/pubspec.yaml (2)
11-11: Workspace flag added – good moveThe
resolution: workspacekey is required for the new pub workspaces feature and is correctly placed at top level.
21-21: Confirmjson_annotation4.9.x is compatible withjson_serializable6.3.2The generator’s minimum required
json_annotationversion can drift; olderjson_serializablereleases occasionally pin lower ranges. Please rundart pub getto ensure there is no version-solve conflict, and bumpjson_serializableif necessary.packages/freezed_annotation/pubspec.yaml (2)
10-12: SDK floor raised & workspace enabled – LGTMThe bump to
>=3.5.0and the workspace resolution flag align this package with the rest of the mono-repo.
15-15: Dependency consistency check
json_annotation: ^4.9.0matches the version used byfreezed; good. Ensure downstream consumers that import onlyfreezed_annotationwill still resolve a compatible version when not using the workspace (e.g., when published).pubspec.yaml (2)
3-4: Confirm SDK upper-bound alignment across workspace packagesThe root workspace pins the SDK with
^3.7.0.
Please double-check each child package’spubspec.yamlso none of them declare an upper-bound lower than 3.7.0; otherwisepub getwill fail.No code change needed if already consistent.
6-12: LGTM – workspace list looks correctDirectories are explicitly enumerated and match the repository layout.
Nothing to add here.packages/freezed/example/test/diagnosticable_test.dart (1)
12-12: Assertion simplification is fineThe single-line
expectkeeps the test readable and behaviour unchanged.packages/freezed/example/test/json_test.dart (2)
6-13: Stylistic change only – no issuesInline map literals make the assertion more concise. 👍
18-18: Likewise, deserialization assertion unchangedNo functional impact; test remains correct.
packages/freezed/example/pubspec.yaml (1)
5-7: Workspace declaration looks good
resolution: workspaceis correctly placed at the root level and the SDK floor is lifted to 3.7 – matches the rest of the PR.packages/freezed/example/lib/main.dart (1)
24-28: Constructor re-formatting only – LGTM
Purely cosmetic change, no behaviour impact.packages/freezed/example/lib/main.g.dart (1)
1-50: Generated file – skipping manual reviewbenchmarks/pubspec.yaml (1)
4-7: Workspace alignment acknowledgedEnvironment bump and workspace flag match the example package – good.
packages/freezed/example/lib/time_slot.freezed.dart (1)
1-146: Generated code – no manual issues detected.github/workflows/build.yml (2)
50-53: Why exclude format checks for the example package?Excluding
packages/freezed/examplefrom format checks seems counterintuitive. Example code should maintain the same formatting standards as the rest of the codebase to provide good examples for users.Is there a specific reason for this exclusion? If it's due to generated files, consider using more granular exclusions or ensuring the code generator produces properly formatted output.
35-37: Good workflow improvements!The following changes improve the CI workflow:
- Setting a global working directory reduces repetition
- Simplified dependency installation and code generation steps
- Using
flutter analyze --fatal-infosensures informational issues are caught- Improved test detection logic properly handles different test frameworks
- Appropriate scoping of lint golden tests
Also applies to: 47-48, 56-57, 60-60, 64-68, 71-72
benchmarks/lib/src/equal.freezed.dart (1)
86-89: Good immutability pattern implementation.The use of
EqualUnmodifiableListViewto wrap the list ensures that the list cannot be modified externally, which is a good practice for immutable data classes.packages/freezed/example/lib/non_diagnosticable.freezed.dart (1)
1-1037: Good addition of comprehensive freezed examples.This generated file provides excellent examples of various freezed patterns including generic classes, mixins, multiple constructors, and different interface implementations. Adding these generated files to version control (as mentioned in the PR context) helps with workspace performance by avoiding regeneration on every build.
|
@rrousselGit I think this works now! 🤞🏻 If so, I'll try to follow up with build v4/source_gen v4/analyzer 8 upgrade shortly after merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
benchmarks/pubspec.yaml (1)
19-20: Missing version for build_runner will break pub get.
build_runner:has no version/spec. Pubspec entries can’t be null; this will fail resolution. Pin a version (align with other packages) or remove it if unused.Apply one of these:
dev_dependencies: freezed: ^3.0.2 json_serializable: ^6.1.3 - build_runner: + build_runner: ^2.4.11Or remove if unneeded:
dev_dependencies: freezed: ^3.0.2 json_serializable: ^6.1.3 - build_runner:
🧹 Nitpick comments (7)
packages/freezed_lint/example/analysis_options.yaml (1)
11-15: Scoped ignores OK; prefer local pragma where feasible.These “expected errors” ignores are fine for examples. If any of these are tied to specific fixtures, consider replacing them with file-level
// ignore_for_file:pragmas to avoid masking unrelated regressions.packages/freezed/build.yaml (1)
14-15: Redundant include pattern.
test/**/*already coverstest/*. You can drop the single-level pattern to simplify.include: - - test/* - test/**/*.github/workflows/build.yml (3)
35-37: Nice: unified working-directory.With
defaults.run.working-directoryset, you can remove per-step working-directory overrides below.- - name: Analyze - run: flutter analyze --fatal-infos - working-directory: ${{ matrix.package }} + - name: Analyze + run: flutter analyze --fatal-infos- - name: Run tests - run: | + - name: Run tests + run: | if grep -q "flutter_test:" "pubspec.yaml"; then flutter test elif grep -q "test:" "pubspec.yaml"; then dart test fi - working-directory: ${{ matrix.package }}
45-45: Cache on both channels (or stable) for speed.Caching only on master likely slows stable runs. Consider caching on both channels, or at least on stable where most contributors run locally.
- cache: ${{ matrix.channel == 'master' }} + cache: true
66-70: Anchor grep for test detection to reduce false positives.Anchor to line starts to avoid matching commented lines or keys like
integration_test:etc.- if grep -q "flutter_test:" "pubspec.yaml"; then + if grep -Eq '^\s*flutter_test\s*:' pubspec.yaml; then flutter test - elif grep -q "test:" "pubspec.yaml"; then + elif grep -Eq '^\s*test\s*:' pubspec.yaml; then dart test fipackages/freezed/example/lib/non_diagnosticable.freezed.dart (2)
16-35: Potentially misleading base mixin equality.The mixin _$Example defines operator == that only checks runtimeType, ignoring fields. It’s harmless if every concrete implementation overrides equality (as done below), but it can be confusing. Consider omitting operator == from the base mixin in future generator versions.
Please confirm no types mix in _$Example without providing their own equality.
840-864: Language features: abstract mixin class with factory.This pattern requires modern Dart. Ensure all packages in the melos workspace have aligned SDK constraints and CI uses a matching Dart/Flutter toolchain.
If CI is flaky locally, verify toolchain versions used by melos/bootstrap vs CI matrix and pin them in the workflow.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
.github/workflows/build.yml(1 hunks).gitignore(1 hunks)benchmarks/.gitignore(0 hunks)benchmarks/lib/src/copy_with.dart(1 hunks)benchmarks/lib/src/copy_with.freezed.dart(1 hunks)benchmarks/lib/src/equal.freezed.dart(1 hunks)benchmarks/pubspec.yaml(1 hunks)packages/freezed/.gitignore(0 hunks)packages/freezed/build.yaml(1 hunks)packages/freezed/example/.gitignore(0 hunks)packages/freezed/example/lib/diagnosticable.freezed.dart(1 hunks)packages/freezed/example/lib/equals.freezed.dart(1 hunks)packages/freezed/example/lib/main.freezed.dart(1 hunks)packages/freezed/example/lib/main.g.dart(1 hunks)packages/freezed/example/lib/non_diagnosticable.freezed.dart(1 hunks)packages/freezed/example/lib/time_slot.freezed.dart(1 hunks)packages/freezed/example/pubspec.yaml(1 hunks)packages/freezed/pubspec.yaml(1 hunks)packages/freezed_annotation/.gitignore(0 hunks)packages/freezed_annotation/pubspec.yaml(1 hunks)packages/freezed_lint/.gitignore(1 hunks)packages/freezed_lint/example/analysis_options.yaml(1 hunks)packages/freezed_lint/example/lib/missing_mixin.freezed.dart(0 hunks)packages/freezed_lint/example/pubspec.yaml(1 hunks)packages/freezed_lint/lib/src/missing_mixin.dart(1 hunks)packages/freezed_lint/lib/src/missing_private_empty_ctor.dart(1 hunks)packages/freezed_lint/pubspec.yaml(1 hunks)packages/freezed_lint/pubspec_overrides.yaml(0 hunks)pubspec.yaml(1 hunks)pubspec_overrides.yaml(1 hunks)
💤 Files with no reviewable changes (6)
- packages/freezed/.gitignore
- packages/freezed_lint/pubspec_overrides.yaml
- benchmarks/.gitignore
- packages/freezed/example/.gitignore
- packages/freezed_lint/example/lib/missing_mixin.freezed.dart
- packages/freezed_annotation/.gitignore
✅ Files skipped from review due to trivial changes (1)
- packages/freezed/example/lib/equals.freezed.dart
🚧 Files skipped from review as they are similar to previous changes (15)
- packages/freezed_lint/lib/src/missing_mixin.dart
- pubspec.yaml
- packages/freezed/example/pubspec.yaml
- packages/freezed_lint/example/pubspec.yaml
- packages/freezed/example/lib/main.g.dart
- .gitignore
- benchmarks/lib/src/copy_with.dart
- packages/freezed_lint/.gitignore
- benchmarks/lib/src/equal.freezed.dart
- benchmarks/lib/src/copy_with.freezed.dart
- packages/freezed_annotation/pubspec.yaml
- packages/freezed_lint/lib/src/missing_private_empty_ctor.dart
- packages/freezed/example/lib/main.freezed.dart
- packages/freezed_lint/pubspec.yaml
- pubspec_overrides.yaml
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-07T20:51:22.701Z
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Applied to files:
packages/freezed_lint/example/analysis_options.yamlbenchmarks/pubspec.yaml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: freezed (packages/freezed_annotation, stable, get)
- GitHub Check: freezed (packages/freezed/example, stable, downgrade)
- GitHub Check: freezed (packages/freezed/example, stable, get)
- GitHub Check: freezed (packages/freezed, master, get)
- GitHub Check: freezed (packages/freezed, stable, get)
- GitHub Check: freezed (packages/freezed, stable, downgrade)
🔇 Additional comments (21)
packages/freezed/pubspec.yaml (2)
11-11: Confirm workspace resolution behavior and SDK alignment.Adding
resolution: workspaceis correct for Pub Workspaces. Please confirm the root workspace includes this package and that CI’s stable SDK version is >= 3.8.0 so workspace resolution and the package’s SDK constraint don’t conflict.
19-22: Version bumps look good; verify transitive compatibility.The updates to source_gen/json_annotation/dart_style look consistent with analyzer (>=7.5.9 <8.0.0). Run a quick
dart pub get && dart pub outdated --resolution=workspacein this package to ensure no hidden conflicts.benchmarks/pubspec.yaml (3)
6-9: Workspace enablement looks right; ensure it resolves to local packages.With
resolution: workspace,freezedandfreezed_annotationshould resolve to local workspace versions that satisfy your constraints. Please confirmdart pub getin benchmarks uses the workspace copies (not hosted).
13-14: Dependency ranges are compatible with workspace versions.
freezed_annotation: ^3.0.0andjson_annotation: ^4.9.0look fine given the workspace. No changes needed.
17-17: Dev dependency range acceptable.
freezed: ^3.0.2will resolve to the workspace’s 3.2.x. No action required.packages/freezed/build.yaml (1)
3-6: Good: exclude example at the target level.Moving
example/*exclusion to$default.sourcesis cleaner and avoids per-builder redundancy..github/workflows/build.yml (1)
32-34: Clarify downgrade intent.Current matrix excludes
channel: masterwithdependencies: downgrade, so downgrade runs only on stable. Confirm that’s intentional; earlier workflows often exercised downgrade on master (nightly) to catch forward-compat issues.packages/freezed/example/lib/time_slot.freezed.dart (9)
1-6: Generated file header looks correct.Standard ignore annotations and coverage exclusion are appropriate for generated code. Nothing to change here.
16-24: Public API surface (start/end, copyWith) looks consistent with Freezed output.Nullable TimeOfDay getters and the eager copyWith getter are fine. No action.
28-35: Equality/hashCode implementations LGTM.Relies on TimeOfDay’s value equality; Object.hash is appropriate.
80-86: _TimeSlot implementation matches the declared API.Constructor and final fields look correct.
90-93: copyWith override on _TimeSlot is consistent.Returns the specialized _$TimeSlotCopyWith; inlining hints are fine.
96-103: Equality/hashCode for _TimeSlot LGTM.Matches the mixin behavior; no divergence.
134-140: copyWith call path is correct.Sentinel handling and casting to TimeOfDay? are as expected.
66-72: No action needed –freezed_annotationimport and part directive are present intime_slot.dart.
45-51: Dart SDK constraints confirmed
All pubspec.yaml files specify a minimum SDK ≥3.5.0 (Dart 3+), so theabstract mixin classsyntax is supported and no changes are required.packages/freezed/example/lib/diagnosticable.freezed.dart (2)
54-63: Good: diagnostics are applied on the concrete type.extends Example with DiagnosticableTreeMixin is the right pattern; once the two “implements” above are removed, this will continue to work as intended (DiagnosticableTreeMixin will call the debugFillProperties defined in the mixin).
Please run dart analyze to confirm there are no remaining missing-implementation warnings for DiagnosticableTree/Diagnosticable.
114-120: The script will verify your SDK constraints and inspect any DiagnosticableTreeMixin or mixin/factory usages. Please review the output and confirm that:
- Every pubspec.yaml’s environment.sdk is set to at least “>=3.0.0” (or your minimum required Dart version).
- No mixin improperly implements DiagnosticableTreeMixin.
- The generated example code’s mixin/factory patterns align with your SDK’s language feature support.
packages/freezed/example/lib/non_diagnosticable.freezed.dart (3)
48-76: Concrete equality/hash for _Example looks correct.Field-wise equality with Object.hash is fine here.
130-142: Deep equality on generic field c is appropriate.Using DeepCollectionEquality for T handles list/map/set payloads. Looks good.
971-999: EnsureHousemixin is declared or imported
NoHousedefinition was found underpackages/freezed/example/lib; add or import its declaration to avoid missing‐symbol errors (note:GeographicAreais already defined).
| if: matrix.package != 'packages/freezed_lint/example' | ||
| continue-on-error: true | ||
| run: dart run build_runner build --delete-conflicting-outputs | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Gate codegen on dependency; drop continue-on-error.
Instead of relying on continue-on-error, conditionally run build_runner only when declared. This avoids masking real failures on packages that should generate code.
- - name: Generate
- if: matrix.package != 'packages/freezed_lint/example'
- continue-on-error: true
- run: dart run build_runner build --delete-conflicting-outputs
+ - name: Generate
+ if: matrix.package != 'packages/freezed_lint/example'
+ run: |
+ if grep -Eq '^\s*build_runner\s*:' pubspec.yaml; then
+ dart run build_runner build --delete-conflicting-outputs
+ else
+ echo "Skipping build_runner (no dependency found)"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if: matrix.package != 'packages/freezed_lint/example' | |
| continue-on-error: true | |
| run: dart run build_runner build --delete-conflicting-outputs | |
| - name: Generate | |
| if: matrix.package != 'packages/freezed_lint/example' | |
| run: | | |
| if grep -Eq '^\s*build_runner\s*:' pubspec.yaml; then | |
| dart run build_runner build --delete-conflicting-outputs | |
| else | |
| echo "Skipping build_runner (no dependency found)" | |
| fi |
🤖 Prompt for AI Agents
In .github/workflows/build.yml lines 56-59, the step currently uses a simple
matrix.package check and continue-on-error to avoid failing runs; instead remove
continue-on-error and gate the run with an explicit condition that only selects
packages that declare codegen (e.g., detect packages that list
build_runner/build_* in dev_dependencies or include a build.yaml) — implement
this by adding a prior job/step that scans package/pubspec.yaml files and emits
an output (a list or map) of packages requiring codegen, then change the step's
if to check membership (for example using
contains(needs.scan.outputs.packages_with_codegen, matrix.package)) and keep
run: dart run build_runner build --delete-conflicting-outputs so failures
surface for packages that should generate code.
| mixin _$Example<T> implements DiagnosticableTreeMixin { | ||
|
|
||
| dynamic get theAnswer; | ||
|
|
||
|
|
||
| @override | ||
| void debugFillProperties(DiagnosticPropertiesBuilder properties) { | ||
| properties | ||
| ..add(DiagnosticsProperty('type', 'Example<$T>')) | ||
| ..add(DiagnosticsProperty('theAnswer', theAnswer)); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Don't “implements DiagnosticableTreeMixin” on a Freezed mixin; use it on concrete classes only.
Using implements on a mixin forces you to re-implement the entire DiagnosticableTree/Diagnosticable interface surface (e.g., toDiagnosticsNode, toStringShort), not just debugFillProperties. This is likely to trip analyzer errors and is unnecessary since concrete variants already mix in DiagnosticableTreeMixin.
Suggested fix: drop the implements here and keep DiagnosticableTreeMixin on the concrete classes.
-mixin _$Example<T> implements DiagnosticableTreeMixin {
+mixin _$Example<T> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mixin _$Example<T> implements DiagnosticableTreeMixin { | |
| dynamic get theAnswer; | |
| @override | |
| void debugFillProperties(DiagnosticPropertiesBuilder properties) { | |
| properties | |
| ..add(DiagnosticsProperty('type', 'Example<$T>')) | |
| ..add(DiagnosticsProperty('theAnswer', theAnswer)); | |
| } | |
| mixin _$Example<T> { | |
| dynamic get theAnswer; | |
| @override | |
| void debugFillProperties(DiagnosticPropertiesBuilder properties) { | |
| properties | |
| ..add(DiagnosticsProperty('type', 'Example<$T>')) | |
| ..add(DiagnosticsProperty('theAnswer', theAnswer)); | |
| } | |
| } |
🤖 Prompt for AI Agents
In packages/freezed/example/lib/diagnosticable.freezed.dart around lines 16 to
26, the generated mixin currently declares "implements DiagnosticableTreeMixin",
which forces re-implementing Diagnosticable/DiagnosticableTree API on the mixin;
remove the "implements DiagnosticableTreeMixin" clause from the _$Example<T>
mixin so it is a plain mixin, and ensure the concrete classes (the
sealed/variant classes) continue to mix in DiagnosticableTreeMixin (or are left
unchanged) so only concrete classes carry the Diagnosticable behavior.
| mixin _$ConcreteExample<T> implements DiagnosticableTreeMixin { | ||
|
|
||
|
|
||
|
|
||
|
|
||
| @override | ||
| void debugFillProperties(DiagnosticPropertiesBuilder properties) { | ||
| properties | ||
| ..add(DiagnosticsProperty('type', 'ConcreteExample<$T>')) | ||
| ; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Same issue: remove implements DiagnosticableTreeMixin from this mixin.
Keep diagnostics on the concrete _ConcreteExample (which already has “with DiagnosticableTreeMixin”). The mixin itself shouldn’t implement the interface.
-mixin _$ConcreteExample<T> implements DiagnosticableTreeMixin {
+mixin _$ConcreteExample<T> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| mixin _$ConcreteExample<T> implements DiagnosticableTreeMixin { | |
| @override | |
| void debugFillProperties(DiagnosticPropertiesBuilder properties) { | |
| properties | |
| ..add(DiagnosticsProperty('type', 'ConcreteExample<$T>')) | |
| ; | |
| } | |
| mixin _$ConcreteExample<T> { | |
| @override | |
| void debugFillProperties(DiagnosticPropertiesBuilder properties) { | |
| properties | |
| ..add(DiagnosticsProperty('type', 'ConcreteExample<$T>')) | |
| ; | |
| } |
🤖 Prompt for AI Agents
In packages/freezed/example/lib/diagnosticable.freezed.dart around lines 198 to
208, the mixin _$ConcreteExample<T> incorrectly declares "implements
DiagnosticableTreeMixin"; remove that implements clause from the mixin
declaration so the mixin does not implement DiagnosticableTreeMixin (leave the
concrete _ConcreteExample<T> class to keep "with DiagnosticableTreeMixin"), and
ensure debugFillProperties remains available on the concrete class without
changing its body.
| working-directory: ${{ matrix.package }} | ||
| if: matrix.package != 'packages/freezed_lint/example' | ||
| continue-on-error: true | ||
| run: dart run build_runner build --delete-conflicting-outputs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| run: dart run build_runner build --delete-conflicting-outputs | |
| run: dart run build_runner build |
I believe that flag is now deprecated, but I'm also coming back to dart after a hiatus, so maybe I'm wrong about that.
This is what Riverpod does, so it's probably fine? It also makes it easier to browse the code on GitHub.
This again... dart-lang/sdk#60784
Mis-merge-conflict-resolved this I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/build.yml (1)
56-58: Gate codegen; don’t mask failures with continue-on-error.Only run build_runner where declared (or build.yaml exists) and fail if it breaks. This mirrors the earlier suggestion.
- - name: Generate - if: matrix.package != 'packages/freezed_lint/example' - continue-on-error: true - run: dart run build_runner build --delete-conflicting-outputs + - name: Generate + if: matrix.package != 'packages/freezed_lint/example' + run: | + if grep -Eq '^\s*build_runner\s*:' pubspec.yaml || test -f build.yaml; then + dart run build_runner build --delete-conflicting-outputs + else + echo "Skipping build_runner (no dependency/build.yaml)" + fi
🧹 Nitpick comments (11)
packages/freezed_lint/example/analysis_options.yaml (2)
11-15: Scope down unused_element ignore to specific filesGlobal ignore in the example can hide accidental dead code. Keep the other expected-error ignores, but move unused_element to per-file via // ignore_for_file to retain signal.
Apply this diff here:
- unused_element: ignoreThen, in the specific example files that intentionally define unused symbols, add at the top (outside this diff’s scope):
// ignore_for_file: unused_element
11-11: Make the intent explicit for future maintainersTighten the comment so folks don’t “fix” these on autopilot.
- # Expected errors + # Expected errors in this example package to exercise freezed_lint rules.github/workflows/build.yml (7)
35-37: Drop redundant working-directory overrides.defaults.run sets working-directory; per-step working-directory is unnecessary here.
- - name: Analyze - run: flutter analyze --fatal-infos - working-directory: ${{ matrix.package }} + - name: Analyze + run: flutter analyze --fatal-infos - - name: Run tests + - name: Run tests run: | if grep -q "flutter_test:" "pubspec.yaml"; then flutter test elif grep -q "test:" "pubspec.yaml"; then dart test fi - working-directory: ${{ matrix.package }}Also applies to: 62-62, 71-71
45-45: Enable cache for all channels (faster CI).No downside to caching on stable too; key already includes channel.
- cache: ${{ matrix.channel == 'master' }} + cache: true
61-61: Use dart analyze for non-Flutter packages.Saves startup time; still fatal-infos.
- - name: Analyze - run: flutter analyze --fatal-infos + - name: Analyze + run: | + if grep -Eq '^\s*sdk\s*:\s*flutter' pubspec.yaml; then + flutter analyze --fatal-infos + else + dart analyze --fatal-infos + fi
66-70: Harden test detection with anchored matches.Avoid false positives from comments/strings.
- if grep -q "flutter_test:" "pubspec.yaml"; then + if grep -Eq '^\s*flutter_test\s*:' pubspec.yaml; then flutter test - elif grep -q "test:" "pubspec.yaml"; then + elif grep -Eq '^\s*test\s*:' pubspec.yaml; then dart test
19-19: Normalize package paths.Minor: benchmarks/ has a trailing slash while others don’t; pick one style.
- - benchmarks/ + - benchmarks - - packages/freezed_lint/example + - packages/freezed_lint/exampleAlso applies to: 24-24
3-11: Optional: add concurrency to cancel superseded runs.Speeds iteration on PRs.
name: Build + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }}-${{ matrix.package }}-${{ matrix.channel }}-${{ matrix.dependencies }} + cancel-in-progress: true
39-46: Pin minimal permissions for Actions.Tighten token scope.
jobs: freezed: runs-on: ubuntu-latest + permissions: + contents: readpackages/freezed/example/lib/main.freezed.dart (2)
221-224: Make$UnionCopyWithabstract to avoid accidental construction.This type is used as an interface. Marking it abstract clarifies intent and prevents instantiation.
- class $UnionCopyWith<$Res> { + abstract class $UnionCopyWith<$Res> { $UnionCopyWith(Union _, $Res Function(Union) __); }
251-269: Align Data’s diagnostics/toString with other variants.Other variants use
Union.loading,Union.error,Union.complex.Datacurrently reportsUnion/Union(...). Consider making it consistent.void debugFillProperties(DiagnosticPropertiesBuilder properties) { properties - ..add(DiagnosticsProperty('type', 'Union')) + ..add(DiagnosticsProperty('type', 'Union.data')) ..add(DiagnosticsProperty('value', value)); } ... @override String toString({ DiagnosticLevel minLevel = DiagnosticLevel.info }) { - return 'Union(value: $value)'; + return 'Union.data(value: $value)'; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (32)
.gitattributes(1 hunks).github/workflows/build.yml(1 hunks).gitignore(1 hunks)benchmarks/.gitignore(0 hunks)benchmarks/lib/src/copy_with.dart(1 hunks)benchmarks/lib/src/copy_with.freezed.dart(1 hunks)benchmarks/lib/src/equal.freezed.dart(1 hunks)benchmarks/pubspec.yaml(1 hunks)packages/freezed/.gitignore(0 hunks)packages/freezed/build.yaml(1 hunks)packages/freezed/example/.gitignore(0 hunks)packages/freezed/example/lib/diagnosticable.freezed.dart(1 hunks)packages/freezed/example/lib/equals.freezed.dart(1 hunks)packages/freezed/example/lib/main.freezed.dart(1 hunks)packages/freezed/example/lib/main.g.dart(1 hunks)packages/freezed/example/lib/non_diagnosticable.freezed.dart(1 hunks)packages/freezed/example/lib/time_slot.freezed.dart(1 hunks)packages/freezed/example/pubspec.yaml(1 hunks)packages/freezed/pubspec.yaml(1 hunks)packages/freezed_annotation/.gitignore(0 hunks)packages/freezed_annotation/pubspec.yaml(1 hunks)packages/freezed_annotation/pubspec_overrides.yaml(0 hunks)packages/freezed_lint/.gitignore(1 hunks)packages/freezed_lint/example/analysis_options.yaml(1 hunks)packages/freezed_lint/example/lib/missing_mixin.freezed.dart(0 hunks)packages/freezed_lint/example/pubspec.yaml(1 hunks)packages/freezed_lint/lib/src/missing_mixin.dart(1 hunks)packages/freezed_lint/lib/src/missing_private_empty_ctor.dart(1 hunks)packages/freezed_lint/pubspec.yaml(1 hunks)packages/freezed_lint/pubspec_overrides.yaml(0 hunks)pubspec.yaml(1 hunks)pubspec_overrides.yaml(1 hunks)
💤 Files with no reviewable changes (7)
- packages/freezed_annotation/pubspec_overrides.yaml
- packages/freezed_lint/example/lib/missing_mixin.freezed.dart
- packages/freezed/example/.gitignore
- packages/freezed_annotation/.gitignore
- benchmarks/.gitignore
- packages/freezed_lint/pubspec_overrides.yaml
- packages/freezed/.gitignore
✅ Files skipped from review due to trivial changes (5)
- packages/freezed_lint/lib/src/missing_mixin.dart
- packages/freezed_lint/lib/src/missing_private_empty_ctor.dart
- benchmarks/lib/src/copy_with.dart
- packages/freezed/example/lib/main.g.dart
- .gitattributes
🚧 Files skipped from review as they are similar to previous changes (17)
- benchmarks/lib/src/copy_with.freezed.dart
- benchmarks/lib/src/equal.freezed.dart
- packages/freezed_lint/example/pubspec.yaml
- benchmarks/pubspec.yaml
- .gitignore
- pubspec.yaml
- packages/freezed/example/pubspec.yaml
- packages/freezed/build.yaml
- pubspec_overrides.yaml
- packages/freezed/example/lib/equals.freezed.dart
- packages/freezed/example/lib/time_slot.freezed.dart
- packages/freezed_annotation/pubspec.yaml
- packages/freezed/example/lib/non_diagnosticable.freezed.dart
- packages/freezed/example/lib/diagnosticable.freezed.dart
- packages/freezed/pubspec.yaml
- packages/freezed_lint/pubspec.yaml
- packages/freezed_lint/.gitignore
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-07T20:51:22.701Z
Learnt from: lishaduck
PR: rrousselGit/freezed#1147
File: packages/freezed_lint/pubspec.yaml:12-12
Timestamp: 2025-01-07T20:51:22.701Z
Learning: The analyzer version update to >=7.0.0 in freezed_lint package is blocked by invertase/dart_custom_lint#300, preventing alignment with freezed package's analyzer requirements.
Applied to files:
packages/freezed_lint/example/analysis_options.yaml
🔇 Additional comments (6)
packages/freezed_lint/example/analysis_options.yaml (1)
17-17: Plugin activation scope is correct
custom_lintis only enabled inpackages/freezed_lint/example/analysis_options.yamland is not enabled at the workspace or root level..github/workflows/build.yml (1)
58-58: Confirm--delete-conflicting-outputsflag support
Manually rundart run build_runner build --help(with the version locked inpubspec.lock) to ensure--delete-conflicting-outputsis still recognized—if it’s been deprecated, remove it; otherwise leave it in place.
.github/workflows/build.yml:58packages/freezed/example/lib/main.freezed.dart (4)
157-186: Robust union JSON dispatch.Switching on
json['custom-key']with a clearCheckedFromJsonExceptionfallback looks good.
306-346: Loading variant serialization and diagnostics look correct.
433-476: Complex variant: fields, copyWith, JSON all consistent.
51-52: SDK constraints and generated parts verified
All packages enforce Dart 3+ andexample/lib/main.dartcorrectly includespart 'main.freezed.dart'andpart 'main.g.dart'with the generated files present.
|
Ok, yay, hit the flutter_test doesn't work with Analyzer 8 again. Back to draft... |
I'd like being able to use my computer while I have Freezed open. On my
816GB Rampotatocomputer of an M14, opening the Freezed folder slows it down to a crawl. This fixes that.Summary by CodeRabbit
Chores
Tests
Lint
Build/Tooling